feat: Add an nc link parser tool - #234
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds a new LangChain tool that parses Nextcloud deep-link URLs into structured identifiers so the agent can route follow-up tool calls appropriately.
Changes:
- Introduces a Nextcloud URL parser with per-app route handling (Files, Talk, Collectives, Deck, Mail, Calendar, Bookmarks, Cookbook, Forms, Tables).
- Exposes the parser as a
parse_nextcloud_urltool viaget_tools(). - Adds tool-selection “hint” metadata based on the parsed app.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
julien-nc
left a comment
julien-nc
left a comment
There was a problem hiding this comment.
The parsing logic looks good.
How does the agent know a link is a Nextcloud one so it would choose to call parse_nextcloud_url for it?
| if key in query and query[key]: | ||
| return query[key][0] |
There was a problem hiding this comment.
check for query[key] being a non-empty list is missing
There was a problem hiding this comment.
🟡 Changes recommended
URL resolution, subdirectory routing, identifier placement, and several tool hints currently produce incorrect behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
ex_app/lib/all_tools/nextcloud_links.py:133
- This anchor only recognizes installations at the domain root. For a valid instance URL such as
https://host/nextcloud/index.php/apps/deck/..., the path starts with/nextcloud/..., so every app-scoped parser falls through as unknown even though the system prompt identifies that base URL as belonging to this instance. Match the final/apps/...route after an optional webroot.
app_match = re.match(r'^/apps/([^/?#]+)(/.*)?$', path)
ex_app/lib/all_tools/nextcloud_links.py:130
message_idis emitted at the result's top level, while all other actionable identifiers and the documented contract place identifiers inids. Consumers inspectingidswill lose the referenced Talk message; include it in the mapping alongside the token.
return done('talk', 'conversation', {'token': m.group(1)},
message_id=_int(msg.group(1)) if msg else None)
ex_app/lib/all_tools/nextcloud_links.py:24
- This hint tells the agent to list rows using
view_id, but the onlylist_rowstool accepts atable_idand calls/tables/{table_id}/rows(all_tools/tables.py:291-307). Passing a view ID as instructed can query the wrong table or fail; do not advertise an unsupported operation.
'tables': 'Use the Tables tools with table_id (or list rows of the view_id).',
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
80492e9 to
8063fb4
Compare
Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
🤖 AI (if applicable)